Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add parsing and prettier support for @example tags in liquid doc #725

Merged
merged 6 commits into from
Feb 4, 2025

Conversation

EvilGenius13
Copy link
Contributor

@EvilGenius13 EvilGenius13 commented Jan 20, 2025

What are you adding in this PR?

References: https://github.com/Shopify/develop-advanced-edits/issues/471

Added support for parsing and formatting @example tags within LiquidDoc blocks.

Feedback

Right now, we aren't controlling whitespace for example tags. If someone wants to write check out. my great example, it would stay in that format. Should trim whitespace or allow people writing examples to have full control?

What's next? Any followup issues?

I want to add prettier support to automatically move @example tags and content below @param as it is the proper flow.

What did you learn?

AST's and CST's are quite puzzle to figure out.

Test

Example.tag.prettier.support.mp4

Before you deploy

  • I included a minor bump changeset
  • My feature is backward compatible

@EvilGenius13 EvilGenius13 marked this pull request as ready for review January 21, 2025 17:57
Copy link
Contributor

jamesmengo commented Jan 28, 2025

LGTM and 🎩 went well, thanks!

We can write those tests in a follow-up.

f39ac31

@jamesmengo jamesmengo self-requested a review January 28, 2025 18:56
@EvilGenius13 EvilGenius13 force-pushed the implement-examplenode branch from c1b702f to c24909d Compare January 31, 2025 20:43
@EvilGenius13 EvilGenius13 added the #gsd:44310 LiquidDoc label Feb 3, 2025
@@ -765,6 +766,14 @@ export interface LiquidDocParamNode extends ASTNode<NodeTypes.LiquidDocParamNode
/** Optional type annotation for the parameter (e.g. "{string}", "{number}") */
paramType: TextNode | null;
}

/** Represents a `@example` node in a LiquidDoc comment - `@example @exampleContent` */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/** Represents a `@example` node in a LiquidDoc comment - `@example @exampleContent` */
/** Represents a `@example` node in a LiquidDoc comment - `@example exampleContent` */

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in #759

Comment on lines +1315 to +1320
exampleContent: {
type: NodeTypes.TextNode,
value: node.exampleContent.value,
position: position(node.exampleContent),
source: node.exampleContent.source,
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
exampleContent: {
type: NodeTypes.TextNode,
value: node.exampleContent.value,
position: position(node.exampleContent),
source: node.exampleContent.source,
},
exampleContent: toTextNode(node.exampleContent),

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you also fix this for paramNode? A follow-up PR is fine too :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in #759
Left out paramNode as it's currently being refactored in another PR

Comment on lines +548 to +551

if (node.exampleContent?.value) {
const content = node.exampleContent.value;
if (content) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (node.exampleContent?.value) {
const content = node.exampleContent.value;
if (content) {
const content = node.exampleContent?.value
if (content) {
....
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in #759

It should respect example content with param and description
{% doc %}
@param paramName - param with description
@example
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think @example should have any empty newline if it's not the first node, wdyt?

Something like this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. This will be tackled in a separate PR as there is some other prettier logic that will need refactoring as well.

Copy link
Contributor

Thanks for the changes! The only blocking comment I have is around this, let me know your thoughts!

@EvilGenius13 EvilGenius13 merged commit 5db0986 into main Feb 4, 2025
6 checks passed
@EvilGenius13 EvilGenius13 deleted the implement-examplenode branch February 4, 2025 17:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
#gsd:44310 LiquidDoc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants